Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: proof_of_sql_parser::intermediate_ast::UnaryOp with sqlparser::ast::UnaryOp in the proof-of-sql crate #363

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

varshith257
Copy link
Contributor

@varshith257 varshith257 commented Nov 9, 2024

Please be sure to look over the pull request guidelines here: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#submit-pr.

Please go through the following checklist

Rationale for this change

This PR addresses the need to replace the proof_of_sql_parser::intermediate_ast::UnaryOp with the sqlparser::ast::UnaryOp in the proof-of-sql crate as part of a larger transition toward integrating the sqlparser .

This change is a subtask of issue #235, with the main goal of streamlining the repository by switching to the sqlparser crate and gradually replacing intermediary constructs like proof_of_sql_parser::intermediate_ast with sqlparser::ast.

What changes are included in this PR?

  • All instances of proof_of_sql_parser::intermediate_ast::UnaryOp have been replaced with sqlparser::ast::UnaryOp
  • Every usage of UnaryOp has been updated to maintain the original functionality, ensuring no changes to the logic or behavior.
  • Any unsupported UnaryOp variants from sqlparser have been appropriately handled using existing error handling mechanisms (i.e., the Unsupported variant in ExpressionEvaluationError).

Are these changes tested?

Yes

Part of #235

@iajoiner
Copy link
Contributor

@varshith257 Let's get #344 merged first so that your PR can be simpler. :)

@iajoiner iajoiner self-requested a review November 11, 2024 14:57
Copy link
Contributor

@iajoiner iajoiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor issues. In general this is excellent! Please complete the PR description and fix these issues.

@iajoiner iajoiner self-requested a review November 11, 2024 17:19
@varshith257 varshith257 force-pushed the fix-unaryop1 branch 4 times, most recently from e5664f3 to 2cf0724 Compare November 11, 2024 19:10
@varshith257 varshith257 changed the title refactor!: PoSQLUnaryOP to use sqlparser::ast::UnaryOP refactor!: proof_of_sql_parser::intermediate_ast::UnaryOp with sqlparser::ast::UnaryOp in the proof-of-sql crate Nov 11, 2024
@varshith257 varshith257 force-pushed the fix-unaryop1 branch 2 times, most recently from 92192fc to 8075a5c Compare November 11, 2024 19:30
@iajoiner iajoiner merged commit ffb40c2 into spaceandtimelabs:main Nov 12, 2024
11 checks passed
Copy link

🎉 This PR is included in version 0.43.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants